Repository navigation
Skip synthesis of candidates whose MPR features or stem names fail - #570
johnml1135 wants to merge 1 commit into
Conversation
After lexical lookup, each alternative is replayed against its root without shapes and dropped when a rule's recorded allomorph requires MPR features the root and earlier trail outputs cannot supply, excludes features the root certainly has, or the rule requires a different stem name. The walk stops at the first violation, runs only when some rule carries such a constraint, and keeps nothing between candidates. Family roots, compounding entries and reachable Overwrite groups disable the unsafe checks. Dropped candidates emit no synthesis trace events. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #570 +/- ##
==========================================
+ Coverage 74.57% 75.00% +0.43%
==========================================
Files 453 454 +1
Lines 38342 38495 +153
Branches 5284 5317 +33
==========================================
+ Hits 28593 28874 +281
+ Misses 8583 8469 -114
+ Partials 1166 1152 -14 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| alternativeCount++; | ||
| if (MaxAlternatives > 0 && alternativeCount > MaxAlternatives) | ||
| throw new MaxAlternativesExceededException("MaxAlternatives exceeded"); | ||
| if (IsPrunedByLookupReplay(alternative)) |
There was a problem hiding this comment.
Minor: F1. With tracing on, a pruned candidate leaves an empty trace node. LexicalLookup (Morpher.cs:466) has already emitted SynthesizeWord for it, but the RequiredStemName/RequiredMprFeatures events that explained the failure are gone. Try-a-Word users see a lookup that ends with no reason. LookupReplayMprTests.cs:42-43 confirms it: they count fewer synthesis events with pruning on under IsTracing = true. The precedent is AnalysisStratumRule.cs:142, which skips merging while tracing; skipping pruning the same way would keep traces unchanged. The parallel loop at line 415 needs the same fix.
| if (upper == null) | ||
| { | ||
| work++; | ||
| upper = lower.Clone(); |
There was a problem hiding this comment.
FYI: F2. upper is a new clone of lower plus extra for every trail entry that has required features, even when extra is still the empty _realizationalOut. In that case lower already is the superset. Using it directly saves one allocation and one work unit per entry. That matters here because the PR's own table puts work/saved at about 1.0 for Aweti and Mbugwe.
| [Test] | ||
| public void NullRecordedAllomorph_IsPrunedOnlyWhenEveryAllomorphFails() | ||
| { | ||
| // Guessed (null) allomorphs happen when the analysis came from a rule whose allomorph is unconstrained; |
There was a problem hiding this comment.
FYI: F3. This comment gives the wrong cause for a null allomorph. AnalysisAffixProcessRule.cs:92 always records _rule.Allomorphs[i], so ParseWord never hands the replay a null allomorph for an affix rule. Nulls come only from GenerateWords/PermuteRules, and that path never calls IsFeasible. The null branches at LookupReplay.cs:145-155 and 167-177 are defensive code. Say that here, so nobody tunes them for a parse case that cannot happen.
Public API, target framework, package, machine.py parity: None verified. Every new member is Findings: F1 new, F2 new, F3 new. Reviewed at c6e2bbc |
|
Closing. It breaks even on real grammars (Aweti +0.07%) and loses on two fixtures, which does not justify 785 lines. The branch stays if a grammar ever shows a real gain. |
Start here:
src/SIL.Machine.Morphology.HermitCrab/LookupReplay.cs--IsMprFeasibleand the check that decides whether a candidate can pass its rules' stem-name and MPR-feature requirements.Status: open; CI green on this head (the
claudejob is skipped).After lexical lookup,
Morpherdrops an alternative before synthesis when a rule needs a stem name or MPR features that its root and trail cannot supply. No parse is lost, and analysis, lookup and the public API are untouched. In total work (analysis events plus synthesis steps) real grammars come out about break-even: Aweti +0.07%, Mbugwe 0.00%. Conformance and inflection-class fixtures lose slightly (-0.45%, -4.77%); a concord-heavy synthetic fixture gains 18%.The unknown a reviewer arrives with is whether this is worth its code at break-even. It removes only work whose output was always discarded, but two fixtures show a net loss once replay work is counted. The question worth the time is whether the filter's soundness holds, because the gain on real grammars is small.
Where to look:
LookupReplay.IsMprFeasible;LookupReplayMprTests,LookupReplayOutputsTests(Any/Overwrite groups, realizational and compounding outputs, parallel path).MaxAlternativescount, per alternative --Morpher.IsPrunedByLookupReplay.Not here:
LookupReplayMode,LookupReplayVerdictandLookupReplayWorkstay because the tests read per-alternative verdicts and work.Verification on this branch:
dotnet test tests/SIL.Machine.Morphology.HermitCrab.Tests -c Releasepassed with 0 failures;dotnet csharpier check .andpwsh ./scripts/comment-hygiene.ps1clean. The shadow-run and step-count measurements are in Evidence. Not run: the full./local_check.sh --agent-strict.Next: approve, or tell me to drop it given the break-even result on real grammars.
Reading this a year from now -- start here
The filter replays, per alternative from lexical lookup, only the checks that need no shapes: required stem names and MPR feature requirements across the rules of the path. If a rule on the path needs something the root plus the trail cannot supply, synthesis of that alternative cannot succeed, so it is skipped. Two MPR feature sets per grammar (realizational and compounding outputs) are built at construction, and the root's set is cloned only when a trail entry adds features.
The research record behind the measurements, including the dropped template replay, is #490 (open).
Evidence and caveats
truncate-morphotactic*fixtures, which also differ between two runs with the filter off.ClassGatedSlots_PruneMostWrongClassCandidatesAndParsesAreUnchangedreproduces the effect as a unit test.Measured work, in steps
Failure-statistics harness (research/failure-reason-stats), one Release build, serial, 60 s per word, words aborted in any arm dropped. Replay work counts one unit per feature-set match, union or copy; units and steps are not shown to cost the same. Analysis events and lookup candidates are unchanged in every row. Net total-work is (steps saved - replay work) over synthesis steps plus analysis events, counting a unit as a step.
The two stress rows are synthetic fixtures not in this PR.
Paths not taken
Template and final-state replay (kept at perf/hc-lookup-replay-templates): Mbugwe +1246 saved for 47,560 work, Aweti +2211 for 3728, Sena 118 for 1825; only Amharic paid (745 for 601). Dropped on cost.
Deferred, and what would unblock it
🤖 Generated with Claude Code
This change is